Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In test cases, use gmtime to avoid timezone & daylight savings issues #12508

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

dereksz
Copy link

@dereksz dereksz commented Feb 2, 2024

This is just a fix to a few test cases; it does not make any changes to the pip source code.

I'm in Sydney, Australia, and just started looking at the pip code for the first time.

First thing I do is clone and run the test via nox. I found test cases failing because of a difference in time stamps found on log messages. Looking further, I see that someone had already had the sense to use time.timezone to adjust the timet style timestamps into local times. However, this adjustment fails when in Sydney as the baked-in date used by the testcases (1547704837.040001 / 2019-01-17T06:00:37,040) falls within daylight savings here, and so the formatted time becomes "2019-01-17T07:00:37,040" - an hour ahead.

I've had enough pain with python, timestamp and time-zones to NOT want to have to make pip a test-bed for all of python's time-zone shortcoming, so the fix in this PR is to have these test cases monkey-patch the root logging.Formatter to use converter=time.gmtime, and to remove the earlier time.timezone adjustments. This now allows the tests cases to pass in southern hemisphere geographies that observe daylight savings.

yield


logging.Formatter.converter = time.gmtime
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be in a fixture instead? Modifying the global state in test seems wrong to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants